Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix ServiceTemplate#picture= with models #18705

Merged
merged 1 commit into from
Apr 30, 2019
Merged

Fix ServiceTemplate#picture= with models #18705

merged 1 commit into from
Apr 30, 2019

Conversation

NickLaMuro
Copy link
Member

When creating a service template with a Picture that already exists (done in some of specs in other repos), the previous form of this method would break as create_picture was not relevant for updating pictures that already exist.

This fix #should™ fix that, but more relevant eyes should probably take a look just to make sure...

Where it fails...

Currently fails in the manageiq-api specs:

Links

Steps for Testing/QA

Try running the API specs with and without this change to the models and confirm that it fails without, and passes with.

@NickLaMuro
Fix ServiceTemplate#picture= with models
1cb062f 
When creating a service template with a Picture that already exists
(done in some of specs in other repos), the previous form of this method
would break as `create_picture` was not relevant for updating pictures
that already exist.

This fix #should™ fix that, but more relevant eyes should probably take
a look just to make sure...
@miq-bot
Copy link
Member

miq-bot commented Apr 29, 2019

Checked commit NickLaMuro@1cb062f with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 1 offense detected

spec/models/service_template_spec.rb

@himdel
Copy link
Contributor

himdel commented Apr 30, 2019

Also fixes UI specs:

  1) CatalogController tests that needs all rbac features access #st_upload_image uploads a selected png file 
     Failure/Error: expect(assigns(:flash_array).first[:message]).to include('Custom Image file "upload_image.png" successfully uploaded')
     
     NoMethodError:
       undefined method `first' for nil:NilClass
     # ./spec/controllers/catalog_controller_spec.rb:287:in `block (4 levels) in <top (required)>'

Finished in 0.30847 seconds (files took 7.25 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/controllers/catalog_controller_spec.rb:284

Thanks!

@martinpovolny martinpovolny merged commit f7fc0cb into ManageIQ:master Apr 30, 2019
@martinpovolny martinpovolny self-assigned this Apr 30, 2019
@himdel
Copy link
Contributor

himdel commented Apr 30, 2019 •
edited
Loading

@miq-bot add_label hammer/yes

@simaishi this fixes travis failures coming from #18689 (hammer/backported)

So we may want to backport ASAP, to get hammer travis green on ui and api.

(triggered ui hammer travis just to be sure it really fails now: https://travis-ci.org/ManageIQ/manageiq-ui-classic/builds/526392856 .. it does fail)

@himdel himdel mentioned this pull request Apr 30, 2019
Closed
simaishi pushed a commit that referenced this pull request Apr 30, 2019
@martinpovolny @simaishi
Merge pull request #18705 from NickLaMuro/fix_service_template_pictur…
40a1d86 
…e_equals

Fix ServiceTemplate#picture= with models

(cherry picked from commit f7fc0cb)

https://bugzilla.redhat.com/show_bug.cgi?id=1702479
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 40a1d86809762479e79f0f654a28b1b8507f4223
Author: Martin Povolny <mpovolny@redhat.com>
Date:   Tue Apr 30 13:31:37 2019 +0200

    Merge pull request #18705 from NickLaMuro/fix_service_template_picture_equals
    
    Fix ServiceTemplate#picture= with models
    
    (cherry picked from commit f7fc0cbfff944f8a39466d77f8c5b1f90551c542)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1702479

@himdel
Copy link
Contributor

himdel commented Apr 30, 2019

@miq-bot remove_label hammer/yes

(forgot editing comments with miq-bot commands in them leads to this :))

@jrafanie
Copy link
Member

It's not super when super looks like return but can't be used in guard clauses. Thanks @NickLaMuro!

@mfeifer mfeifer added this to the Sprint 110 Ending Apr 29, 2019 milestone Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@martinpovolny martinpovolny

Labels
bug hammer/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@NickLaMuro @miq-bot @himdel @simaishi @jrafanie @martinpovolny @mfeifer